-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(): Bevy 0.11 migration #45
Conversation
…lly derived within the Reflect derive macro
Looking forward to this. Not sure how to help, I made an attempt to do the same recently but stuck at a few errors that I couldn't really understand. Sharing the link here for "reference" https://github.com/hafiidz/bevy_proto/tree/upgrade_bevy_0_11 |
Thanks for helping out! Could you put this in draft mode until you’re ready for review? |
Sure ! Sorry about that, it is now in draft mode |
Wow , you have already done pretty much all the work ! |
I think we can just continue here. When I did mine it wasn't with the idea to publish it, so I didn't pay attention to be sticking to proper formatting. Just doing things as I learn from error reports. Maybe you can cherry pick changes as necessary. I'm not sure how to add changes here though. Should I send a PR to your branch separately? Based on brief Google search, it seems to show only the PR requester and repo owner can make changes here. |
Yeah generally in these cases you'd make a PR to their branch. I'll also try to help out with the migration if I find time this week. |
… improvements Chore: Migration 0.10 to 0.11 1. Flatten UI Style properties that use Size + remove Size 2. MeasureFunc improvements Need second eye especially on line 92 to 111 (pub struct ContentSize .... )
Added a PR kaosat-dev#2 to @kaosat-dev v0.11 draft branch |
Chore: Bevy v0.10 to v0.11 cherry picked changes
Thanks a lot ! Went over the changes, and it looks good and matches the needed changes outlined in the migration guide ! |
No worries, all good. Take your time, it's volunteer work anyway. I am just taking opportunity to use Rust in actual project and contributing back to the community since I love using this bevy add on in my personal project. |
Agreed, don't feel rushed to do anything. Like @hafiidz said, this is all on a volunteer basis. You both are already doing more than enough by even taking the time to help out, so thank you! 😄 |
@MrGVSV & @hafiidz thanks for the kind words, also glad to help ! |
No worries, taking a look now! |
Okay here's a pretty major problem: generic types that require an input. Currently, the macro will generate an input that looks like this: #[derive(Schematic)]
struct MyStruct<T, U: Asset> {
foo: T,
#[schematic(asset)]
bar: Handle<U>
}
// Generates:
#[derive(Reflect)]
struct MyStructInput<T> {
foo: T,
#[reflect(ignore, default)]
__phantom_ty__: ::core::marker::PhantomData<fn() -> ( T, U )>,
} By adding the bevyengine/bevy#9046 would hopefully fix this, but I think it won't be until Bevy 0.12. In the meantime, I think we might be able to solve this by adding a temporary attribute to indicate which generics are needed on the input (if one is to be generated): // Struct where attribute needed:
#[derive(Schematic)]
#[schematic(input(generics(T)))] // We don't need `U`
struct MyStruct<T, U: Asset> {
foo: T,
#[schematic(asset)]
bar: Handle<U>
}
// Struct where no attribute needed:
#[derive(Schematic)]
struct AnotherStruct<T> {
foo: T,
} Thoughts on this? |
Haha, yup. A lot of Rust macros really felt like magic to me still. @MrGVSV, unfortunately, I don't have a firm grasp on Rust/Bevy yet to give a strong opinion. It does looks good & especially simple when no attribute needed. |
Not sure if this is the right place, but I think the bevy dependency might need to be more specific, i.e. only compatible with version 0.11.x The following line in, might need changes from Line 103 in e899c10
|
Okay I think I have it fixed. Turns out the issue was with doing |
Could one or both of you test the branch now? Hopefully it should work in your own projects again! |
Thanks ! I will take it for a spin now :) |
ok, wow, so was it a dual issue ? Ie the one with the generic types and the one with defaults ? |
Same as @hafiidz , my understanding of Rust & Bevy is not yet good enough to make an informed decision, ( |
@MrGVSV tested out the branch with your changes, it all works great ! (tried out all examples from the repo+ one of my own) well done 👍 !! |
My personal project might take a day to complete migration, will update tomorrow. At the same time, I have just performed a quick simple project to test, it seems to be working great. However, there is a separate issue with button bundle that might need your help to see later, that still remain persistent even with bevy 0.11 (#44). |
Just take the time to complete the migration of my personal project, much faster than expected. It is running really well now. Thank you very much! Awesome work @MrGVSV. |
Everything seems to be working well now. The error with button bundle is resolved. |
Awesome, I'm going to go ahead and merge this then! Thank you both for your help! |
Hi !
Started working on the migration to Bevy 0.11 , not quite done yet :)